Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore embedded scripts when asked #234

Merged
merged 7 commits into from
Sep 1, 2015
Merged

Ignore embedded scripts when asked #234

merged 7 commits into from
Sep 1, 2015

Conversation

gjtorikian
Copy link
Owner

Closes #233.

This PR proposes to add a new option, ignore_script_embeds, to resolve errors that read Element script embeds close tag.

I'm not 100% sure this is still the best way to handle this, but it might be the only way. From what I read in Nokogiri's docs, there's pretty much no information given from a parse error.

/cc @akshayrawat

@@ -1,6 +1,9 @@
# encoding: utf-8

class HtmlCheck < ::HTML::Proofer::CheckRunner
class HtmlCheckable < ::HTML::Proofer::Checkable

end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this class HtmlCheckable here?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I added it while testing something out. It's unnecessary.

@akshayrawat
Copy link

It would be nice if html-proofer supported multiple html validators like vnu. My current setup disables html validation via proofer and instead uses vnu for that.

@gjtorikian
Copy link
Owner Author

It would be nice if html-proofer supported multiple html validators like vnu. My current setup disables html validation via proofer and instead uses vnu for that.

That's a pretty good idea. It might be a pain to integrate a Java executable into this Ruby library though.

@@ -0,0 +1,3 @@
<script type="text/html" id="navbar-logged-in" data-proofer-ignore>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove data-proofer-ignore from this fixture. The spec “ignores embeded scripts when asked” must pass without the attribute.

@geobrando
Copy link
Contributor

Hey @gjtorikian. Was wondering if there's anything currently in the way of merging this.

gjtorikian added a commit that referenced this pull request Sep 1, 2015
Ignore embedded scripts when asked
@gjtorikian gjtorikian merged commit b206f8d into master Sep 1, 2015
@gjtorikian gjtorikian deleted the ignore-scripts branch September 1, 2015 08:41
@gjtorikian
Copy link
Owner Author

@geobrando Sorry! This is now out in 2.4.0: https://github.com/gjtorikian/html-proofer/releases/tag/v2.4.0

@geobrando
Copy link
Contributor

Thanks!

@geobrando
Copy link
Contributor

@gjtorikian

I'm wondering if I'm missing something. Either I don't know what I'm doing or there is something wrong with the tests.

  ruby:
    interpreter:  "ruby"
    version:      "2.1.2p95"
    date:         "2014-05-08"
    platform:     "i686-linux"
    patchlevel:   "2014-05-08 revision 45877"
    full_version: "ruby 2.1.2p95 (2014-05-08 revision 45877) [i686-linux]"

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'
  gem 'html-proofer', github: 'gjtorikian/html-proofer', :tag => 'v2.4.2'
end    

HTML::Proofer.new("./files",{:check_html => true, :ignore_script_embeds => true}).run

running with the fixture in /files yields

- ./files/repo-test.html
  *  Element script embeds close tag (line 2)
  *  Element script embeds close tag (line 2)
/home/geobrando/.rvm/gems/ruby-2.1.2/bundler/gems/html-proofer-f1f645f19a87/lib/html/proofer.rb:190:in `print_failed_tests': HTML-Proofer found 2 failures! (RuntimeError)
    from /home/geobrando/.rvm/gems/ruby-2.1.2/bundler/gems/html-proofer-f1f645f19a87/lib/html/proofer.rb:92:in `run'
    from /home/geobrando/Projects/htmlproofer-testing/test.rb:8:in `<main>'

@geobrando
Copy link
Contributor

Hey @gjtorikian. Is this working as intended?

@colemanm
Copy link

I'm also still seeing the Element script embeds close tag errors when I have this new option enabled, running 2.5.1 latest. Same as @geobrando with check_html and ignore_script_embeds both true.

@gjtorikian
Copy link
Owner Author

The documentation is wrong, but the feature is working. The options should be passed like this:

opts = { :check_html => true, :validation => { :ignore_script_embeds => true } }

That is, within the :validation namespace (because I anticipated future options).

@geobrando
Copy link
Contributor

@gjtorikian Ah. You are correct. Looking at the binary, I think something needs to be changed to produce the proper namespaced option when using --ignore-script-embeds on the CL but I couldn't figure out how to do it.

From my limited knowledge of ruby it looks like you're mapping the CL flags directly to options that you pass to HTML:proofer. I'll let you figure out how to do this as I assume you'd have to take into account adding additional options in the :validation namespace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ignore script templates
5 participants